Fix summary case card issues. (PT-188001359) #1526
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1526 +/- ##
==========================================
- Coverage 85.34% 85.32% -0.02%
==========================================
Files 570 571 +1
Lines 28496 28549 +53
Branches 7814 7330 -484
==========================================
+ Hits 24319 24360 +41
- Misses 3873 4033 +160
+ Partials 304 156 -148
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
codap-v3
|
||||||||||||||||||||||||
| Project |
codap-v3
|
| Run status |
|
| Run duration | 09m 14s |
| Commit |
|
| Committer | lublagg |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
30
|
Skipped
|
0
|
Passing
|
228
|
lublagg
changed the title
* Splits header into two lines. There is a bug with editing attribute header text. * Fixes bug where header doesn't update after rename * Adds logic so that line 1 always shows 1 word Adds css styling so line doesn't wrap word, and line 2 shows ellipsis on left side. * fix merge conflict error * Fixes broken cypress test in table * Fixes logic for showing attribute units as undefined if there are no units * Adds padding buffer to button width Adds logic that if there is only one attribute name and it overflows, don't make it appear in the 2nd line. * Adds wrap styling for one word headers * PR fixes * Adds code to styling of attribute header line 2
* Adds formula column background color * Adds formula background styling to case card value cell. Disables value editing when attribute has a formula * Adds Cypress test to verify background color when attribute is a formula Splits case card test to 2 context so mouseSensor does not interfere with attribute menu clicks * PR fix
* Adds a pointer capture on component drag * remove commented out code * PR fix
…ax highlighting (#1528) * feat: add CodeMirror-based formula editor with auto-complete and syntax highlighting * chore: fix cypress tests * chore: fix highlighting of backtick identifiers * chore: code review tweaks
There was a problem hiding this comment.
This is looking good to me 👍 Changes to the card code make sense and everything seems to work well in the browser.
I left a few comments that I don't think should hold up merging this in, but I do wonder about all the changes here. I think something got a little mixed up with the commit history. Did you maybe merge main into your branch? I'm not sure but think Kirk may want to clean that up before this gets merged in.
There are also conflicts reported in a few files that will need to be fixed.
v3/cypress/e2e/text.spec.ts
Outdated
| cy.wait(100) | ||
| toolbar.getUndoTool().click() | ||
| cy.wait(100) | ||
| c.getComponentTitle(kTextTileTestId).should("have.text", textDefaultTitle) | ||
|
|
||
| // Redo title change | ||
| toolbar.getRedoTool().should("be.enabled") | ||
| toolbar.getRedoTool().click() | ||
| cy.wait(100) |
There was a problem hiding this comment.
Would it make sense to take out these added waits since they don't seem to have helped and the test is now disabled?
There was a problem hiding this comment.
I agree that the waits should be removed unless they've been determined to be necessary (in which case there's probably a better solution anyway).
There was a problem hiding this comment.
I didn't add these in -- these changes came in when I merged in main. I think if they're on main then maybe it would make sense to make another PR to get rid of them, since it's not necessarily pertinent to this work.
Conflicts: v3/build_number.json v3/cypress/e2e/case-card.spec.ts v3/src/models/data/data-set.ts v3/src/models/data/filtered-cases.ts
I merged from |
There was a problem hiding this comment.
👍 Looks good -- I added a commit which merged main into the PR branch, simplified the summarizedCollections() computation, and tweaked a couple of other things. One thing to note is that if there is only one case in a collection, it needn't be summarized even if nothing is selected.
|
I gave my approval, but @emcelroy's comments should still be considered/addressed. |
Addresses the following issues:
I also noticed some other bugs while working on the above that were all tied to how we were keeping track of the
summarizedCollectionsand whether or not to show the summary view. Instead of having the components pass back information to the model to update thesummarizedCollectionsprop on the Case Card model,summarizedCollectionsis now a view on the model that updates whenever there is a change in the selected cases.